Implement optimized $populate operation with modular design - #95
Merged
Conversation
- Add `FHIRQuestionnairePopulateService` for expression-based `$populate` implementation, enabling generation of `QuestionnaireResponse` from `Questionnaire` and contextual data. - Introduce `PopulateContext` and `PopulateResult` classes to handle operation inputs and outputs. - Add `PopulateModelFactory` for version-specific object construction in `$populate`. - Include initial support for `launchContext` resources and `initialExpression` evaluation with FHIRPath expressions. - Add unit tests and sample fixtures to validate `Questionnaire/$populate` behavior.
… item repetition, and observation-based population - Add support for resolving `variable` extensions as external constants in FHIRPath evaluation. - Implement `itemPopulationContext` handling for repeating group items based on context results. - Enable observation-based population using `observationLinkPeriod`, selecting the most recent matching `Observation`. - Add temporal coercion support (`date`, `dateTime`, `time`) and strengthen answer type compatibility checks. - Refactor `FHIRQuestionnairePopulateService` to modularize `item` population, improve error handling, and centralize value mismatches. - Update `PopulateModelFactory` with temporal wrapper methods, enabling version-specific date/time construction. - Enhance population logic with fallback handling for unresolved canonical URLs.
…uration warnings and expanded diagnostics - Introduce warnings for `Duration` units that cannot be mapped in `observationLinkPeriod`, treating look-back windows as unbounded but making diagnostic issues observable. - Expand test coverage to validate diagnostic issue generation for unmappable Duration units and no-match Observations. - Refactor internal population methods to support comprehensive result and issue inspection in test cases.
…bilities - Add documentation for expanded `$populate` operation, including updated handling for `launchContext`, `variable`, `itemPopulationContext`, and observation-based population (`observationLinkPeriod`). - Enhance complete phpdoc coverage for constructors and methods in public API files. - Expand test suite with new scenarios: malformed expressions, empty results, type mismatches, and nested group repetition. - Incorporate conformance corpus documentation and triage details for SDC IG example forms against implemented functionality. - Refactor test utilities for streamlined validation of population results and issues.
- Introduce `FhirPrimitiveReader` to handle primitive-wrapper-or-scalar value extraction across `Sdc` components. - Add `AnswerValueCoercer` for modular, reusable answer coercion in `FHIRQuestionnairePopulateService`. - Refactor `FHIRQuestionnairePopulateService` to delegate primitive handling and answer coercion to respective helpers. - Remove redundant coercion methods from `FHIRQuestionnairePopulateService`. - Improve type safety, error diagnostics, and code maintainability with centralized logic.
…based population logic - Moved observation-based population functionality from `FHIRQuestionnairePopulateService` to a new `ObservationSelector` class, centralizing handling of `observationLinkPeriod`. - Simplified `FHIRQuestionnairePopulateService` by delegating observation-based population to `ObservationSelector`, reducing duplication and improving maintainability. - Enhanced testability and modularity with clearly defined responsibilities for observation selection and diagnostics.
…ResponseReader` - Add `QuestionnaireResponseReader` for tolerant structural reads of `QuestionnaireResponse`/`Questionnaire` items and answers. - Remove redundant read methods and inline the new reader in `FHIRQuestionnaireResponseExtractService`. - Delegate QR-specific reads (e.g., `childItems`, `answersOf`, `answerValue`) to `QuestionnaireResponseReader`, improving modularity and maintainability. - Simplify `DefinitionExtractionWalker` initialization with shared reader dependency.
…g and extraction logic - Extract item processing into new private methods (`walkOneItem`, `writeAnswerLeaves`, `applyOneExtractValue`, `resolveExtractValue`) for improved readability and maintainability. - Simplify control flow and structure error handling with early returns, reducing nesting and duplication. - Enhance separation of concerns for path processing, value resolution, and answer extraction.
Benchmark ResultsSerialization + FHIRPath benchmarks✅ Regression check passedComparison vs baselineRun |
✅ FHIR Model Generation Test SUCCESSThis automated test regenerates the R4B FHIR models and verifies that the generation functionality works correctly with the changes in this PR, then checks that the committed models are up to date. 📋 Click to view detailed output🎉 The FHIR model generation completed successfully and the committed models are up to date. This comment was automatically generated by the GitHub Actions workflow. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements an offline-first, modular $populate engine in the SDC component, adding a new FHIRQuestionnairePopulateService (with cross-version support) plus extensive conformance + deterministic unit coverage, while also refactoring parts of the $extract implementation into smaller collaborators.
Changes:
- Added expression-based
$populate(launchContext + initialExpression, variables, itemPopulationContext repeating groups) and observation-based population (observationLinkPeriod) with modular helpers. - Added conformance tests backed by frozen reference-engine oracles plus deterministic unit tests for edge cases and observation-based behavior.
- Refactored
$extractinternals intoExtract\*collaborators (walker/reader/factories) and updated docs/readme accordingly.
Reviewed changes
Copilot reviewed 47 out of 47 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Component/Sdc/tests/Unit/TemplateExtractorTest.php | Updates imports to new Sdc\Extract\* namespaces. |
| src/Component/Sdc/tests/Unit/FHIRQuestionnairePopulateServiceTest.php | New deterministic unit tests for $populate behaviors and diagnostics. |
| src/Component/Sdc/tests/Unit/FHIRQuestionnaireObservationPopulateTest.php | New deterministic unit tests for observation-based population (observationLinkPeriod). |
| src/Component/Sdc/tests/Unit/FHIRPathExternalConstantSpikeTest.php | New spike tests validating prerequisite FHIRPath behaviors on deserialized models. |
| src/Component/Sdc/tests/Unit/DefinitionPathWriterTest.php | Updates import to new Sdc\Extract\DefinitionPathWriter namespace. |
| src/Component/Sdc/tests/SOURCES.md | Documents $populate oracle provenance, probes, and corpus decisions. |
| src/Component/Sdc/tests/Integration/Populate/FHIRPopulateConformanceTest.php | New reference-seeded conformance tests for $populate (R4/R4B/R5 parity). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-variables-coercion.questionnaire.json | New $populate oracle input fixture (variables + primitive coercion). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-variables-coercion.patient.json | New $populate oracle input fixture (patient). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-variables-coercion.expected-qr.json | New frozen oracle expected QR (forms-lab output). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-launchcontext-initial.questionnaire.json | New $populate oracle input fixture (launchContext + initialExpression). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-launchcontext-initial.patient.json | New $populate oracle input fixture (patient). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-launchcontext-initial.expected-qr.json | New frozen oracle expected QR (forms-lab output). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-itempopulationcontext.questionnaire.json | New $populate oracle input fixture (itemPopulationContext). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-itempopulationcontext.patient.json | New $populate oracle input fixture (patient). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-itempopulationcontext.expected-qr.json | New frozen oracle expected QR (forms-lab output). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-enablewhen-notsuppressed.questionnaire.json | New $populate oracle input fixture (enableWhen not suppressing population). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-enablewhen-notsuppressed.patient.json | New $populate oracle input fixture (patient). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-enablewhen-notsuppressed.expected-qr.json | New frozen oracle expected QR (forms-lab output). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-reference.questionnaire.json | New $populate oracle input fixture (reference coercion). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-reference.patient.json | New $populate oracle input fixture (patient). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-reference.expected-qr.json | New frozen oracle expected QR (forms-lab output). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-quantity.questionnaire.json | New $populate oracle input fixture (quantity coercion; multi launchContext). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-quantity.patient.json | New $populate oracle input fixture (patient). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-quantity.observation.json | New $populate oracle input fixture (observation). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-quantity.expected-qr.json | New frozen oracle expected QR (forms-lab output). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-coding-marital.questionnaire.json | New $populate oracle input fixture (coding coercion). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-coding-marital.patient.json | New $populate oracle input fixture (patient). |
| src/Component/Sdc/tests/Fixtures/Populate/populate-coercion-coding-marital.expected-qr.json | New frozen oracle expected QR (forms-lab output). |
| src/Component/Sdc/src/PopulateResult.php | New result container for populated QR + optional OperationOutcome. |
| src/Component/Sdc/src/PopulateContext.php | New context object defining version, launch context, subject, and data provider. |
| src/Component/Sdc/src/Populate/PopulateModelFactory.php | New version-scoped factory for QR, answers, and OperationOutcome model construction. |
| src/Component/Sdc/src/Populate/ObservationSelector.php | New isolated observation-based population selection logic. |
| src/Component/Sdc/src/Populate/FhirPrimitiveReader.php | New tolerant primitive/scalar reading utilities for deserialized models. |
| src/Component/Sdc/src/Populate/AnswerValueCoercer.php | New coercion logic mapping expression results into correct answer value[x] shapes. |
| src/Component/Sdc/src/FHIRQuestionnaireResponseExtractService.php | Refactors $extract to use new reader/walker collaborators and updated namespaces. |
| src/Component/Sdc/src/FHIRQuestionnairePopulateService.php | New main $populate service (FHIRPath evaluation, variables, repetition, diagnostics). |
| src/Component/Sdc/src/Extract/TemplateExtractor.php | Moves TemplateExtractor into Sdc\Extract namespace and marks internal. |
| src/Component/Sdc/src/Extract/QuestionnaireResponseReader.php | New tolerant structural reader for $extract over QR/Q items/answers. |
| src/Component/Sdc/src/Extract/ExtractModelFactory.php | Moves ExtractModelFactory into Sdc\Extract namespace and marks internal. |
| src/Component/Sdc/src/Extract/DefinitionPathWriter.php | Moves DefinitionPathWriter into Sdc\Extract namespace and marks internal. |
| src/Component/Sdc/src/Extract/DefinitionExtractionWalker.php | New walker extracting definition-based resources (factored out of service). |
| src/Component/Sdc/src/Contract/PopulationDataProviderInterface.php | New interface seam for supplying observations to offline population. |
| src/Component/Sdc/src/Contract/PopulateServiceInterface.php | New public-ish interface for population services. |
| src/Component/Sdc/src/Contract/ExtractServiceInterface.php | Moves ExtractServiceInterface into Sdc\Contract namespace. |
| src/Component/Sdc/src/BundlePopulationDataProvider.php | New bundle-backed PopulationDataProvider implementation. |
| src/Component/Sdc/README.md | Updates SDC docs to describe supported $populate mechanisms and boundaries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| } | ||
|
|
||
| return [$seconds !== null ? time() - $seconds : null, time()]; |
Owner
Author
There was a problem hiding this comment.
Good call — captured now once so the window's start and end share a single reference instant. (The ordering already made end < start impossible, since end used the later time() call, but capturing once is cleaner and easier to reason about/test.)
Comment on lines
+7
to
+25
| use Ardenexal\FHIRTools\Component\Sdc\FHIRQuestionnaireResponseExtractService; | ||
| use Ardenexal\FHIRTools\Component\Sdc\Populate\FhirPrimitiveReader; | ||
|
|
||
| /** | ||
| * Tolerant structural reads over deserializer-origin `QuestionnaireResponse` / `Questionnaire` item | ||
| * objects for the `$extract` path: link ids, child items, answers, and answer values, plus the | ||
| * string/expression primitive reads extraction relies on. | ||
| * | ||
| * Deliberately stricter than the populate-side {@see FhirPrimitiveReader}: `stringifyPrimitive` reads | ||
| * *only* string-valued primitives (a non-string — e.g. a boolean/integer FHIRPath scalar at | ||
| * `resolveFullUrl` — yields null, driving extraction's `urn:uuid` fallback). The two are NOT | ||
| * interchangeable; see the sdc-service-decomposition M03 reconciliation note. | ||
| * | ||
| * Shared by {@see FHIRQuestionnaireResponseExtractService} and {@see DefinitionExtractionWalker} so both | ||
| * read QR structure identically. All reads use `property_exists` + `?? ` isset-semantics so an | ||
| * uninitialized typed property reads as absent rather than throwing (the model-initialization footgun). | ||
| * | ||
| * @internal implementation detail of the `Sdc` extraction path; not part of the public API | ||
| */ |
Owner
Author
There was a problem hiding this comment.
These imports are referenced from {@see} PHPDoc tags, which PHP-CS-Fixer's no_unused_imports (part of the Pint symfony preset run in CI) treats as usage — so they're not stripped and composer lint passes with them present. Keeping the import plus a short {@see} also matches the convention already shipped in the $extract files (e.g. TemplateExtractor importing FHIRPathService); fully-qualifying only here would be inconsistent. Leaving as-is.
Benchmark ResultsSerialization + FHIRPath benchmarks✅ Regression check passedComparison vs baselineRun |
…pulation` - Implement strict subject matching in `ObservationSelector`, ensuring only `Observations` confirmably about the specified subject are eligible for population. - Raise warnings for mismatched or unconfirmable subjects when `PopulateContext::$subject` is set. - Update population logic, tests, and documentation to reflect subject-scoping behavior. - Prevent data leakage from mixed-subject Bundles, aligning with `observationLinkPeriod` intent.
Benchmark ResultsSerialization + FHIRPath benchmarks✅ Regression check passedComparison vs baselineRun |
… timestamp calculations` - Interpret timezone-less FHIR strings (e.g., date-only) as UTC for deterministic window comparisons. - Prevent race condition in `ObservationSelector` by capturing `time()` once, ensuring consistent window boundaries.
Benchmark ResultsSerialization + FHIRPath benchmarks✅ Regression check passedComparison vs baselineRun |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of Changes
This pull request introduces significant enhancements and modularization to the
$populateoperation within theSdcmodule, laying a robust foundation for maintaining and scaling functionality.Key Features and Improvements
Modularization and Refactoring:
DefinitionExtractionWalkerandobservation-based population, reducing code duplication and improving readability.QuestionnaireResponseReader.New Functionalities:
$populateoperation, including:observationLinkPeriod.FhirPrimitiveReader) and answer coercion (AnswerValueCoercer).Expanded Testing and Documentation:
$populatedocumentation to reflect new and expanded capabilities.Improved Diagnostics:
Durationunits inobservationLinkPeriod, ensuring better tracking of issues.Benefits
These changes collectively improve code maintainability, testability, and extend functionality for
$populateoperations, supporting complex use cases with a modular and scalable approach.